Support Databricks 12.2 LTS and disallow unsupported reader versions and features in Delta Lake by ebyhr · Pull Request #16905 · trinodb/trino

ebyhr · 2023-04-06T04:21:27Z

Description

Support Databricks 12.2 LTS and disallow unsupported reader versions and features in Delta Lake
Fixes #16884

Release notes

(x) Release notes are required, with the following suggested text:

# Delta Lake
* Add support for Databricks 12.2 LTS. ({issue}`16905`)
* Disallow reading tables with [deletion vectors](https://github.com/delta-io/delta/blob/master/PROTOCOL.md#deletion-vectors). Previously, it returned incorrect results. ({issue}`16884`)

findinpath

LGTM

findinpath · 2023-04-14T05:02:25Z

List only the files under _change_data . In this way you can keep the initial assertion

Assertions.assertThat(s3.listObjectsV2(bucketName, changeDataPrefix).getObjectSummaries()).hasSize(0);

List<S3ObjectSummary> summaries = s3.listObjectsV2(new ListObjectsV2Request() .withBucketName(bucketName) .withPrefix(changeDataPrefix) .withDelimiter("/")) .getObjectSummaries();

We could use a constant for "/" - PATH_SEPARATOR

Reference implementation:

trino/plugin/trino-hive/src/main/java/io/trino/plugin/hive/s3/TrinoS3FileSystem.java

Lines 811 to 814 in aa95af1

ListObjectsV2Request request = new ListObjectsV2Request()

.withBucketName(getBucketName(uri))

.withPrefix(key)

.withDelimiter(recursive ? null : PATH_SEPARATOR)

List only the files under _change_data . In this way you can keep the initial assertion

It would be better to avoid hiding the fact of _change_data directory existence in my opinion.

We could use a constant for "/" - PATH_SEPARATOR

Disagreed. There's no benefit to replace the single character (it's a path separator obviously) with such a constant.

Do note that keeping _change_data may be related to

trino/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/procedure/VacuumProcedure.java

Lines 228 to 230 in 513d193

// Ignore directories (and symlinks, etc.). If a partition directory is old and empty (or made empty by us), removing it could break concurrent writes

// on file systems with directories (e.g. HDFS).

// TODO Note: Databricks can delete directories during vacuum on s3. This might need to be revisited.

#12018

Probably not relevant though - as this is relevant for HDFS

findepi · 2023-04-17T11:34:28Z

+        if (protocolEntry.getMinReaderVersion() > MAX_READER_VERSION) {
+            LOG.debug("Skip %s because the reader version is unsupported: %d", dataTableName, protocolEntry.getMinReaderVersion());
+            return null;
+        }
+        Set<String> unsupportedReaderFeatures = unsupportedReaderFeatures(protocolEntry.getReaderFeatures().orElse(ImmutableSet.of()));
+        if (!unsupportedReaderFeatures.isEmpty()) {
+            LOG.debug("Skip %s because the table contains unsupported reader features: %s", dataTableName, unsupportedReaderFeatures);
+            return null;
+        }


We should fail instead or reporting "table not found".

cla-bot Bot added the cla-signed label Apr 6, 2023

github-actions Bot added delta-lake Delta Lake connector docs tests:hive labels Apr 6, 2023

ebyhr force-pushed the ebi/delta-databricks-12.2 branch 6 times, most recently from 9804a65 to b2fe20a Compare April 10, 2023 04:11

ebyhr marked this pull request as ready for review April 11, 2023 03:20

ebyhr force-pushed the ebi/delta-databricks-12.2 branch 2 times, most recently from cb639f4 to 96a34cd Compare April 11, 2023 03:33

ebyhr requested review from findinpath and krvikash April 11, 2023 03:34

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from 96a34cd to 50c321e Compare April 11, 2023 05:32

findinpath reviewed Apr 11, 2023

View reviewed changes

ebyhr force-pushed the ebi/delta-databricks-12.2 branch 2 times, most recently from fac5148 to 30c8056 Compare April 12, 2023 09:38

findinpath reviewed Apr 12, 2023

View reviewed changes

Comment thread docs/src/main/sphinx/connector/delta-lake.rst Outdated

Comment thread plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeSplitManager.java Outdated

ebyhr force-pushed the ebi/delta-databricks-12.2 branch 2 times, most recently from 47047bf to daf7173 Compare April 13, 2023 01:23

ebyhr mentioned this pull request Apr 13, 2023

Support DML operations on Delta tables with name column mapping #16183

Merged

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from daf7173 to 196027c Compare April 13, 2023 08:21

findinpath reviewed Apr 13, 2023

View reviewed changes

Comment thread ...duct-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksDelete.java Outdated

Comment thread plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java Outdated

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from 196027c to 813d33d Compare April 13, 2023 08:31

findinpath approved these changes Apr 13, 2023

View reviewed changes

ebyhr requested a review from findepi April 13, 2023 08:50

findepi approved these changes Apr 13, 2023

View reviewed changes

Comment thread ...sts/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeAlterTableCompatibility.java Outdated

Comment thread ...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeWriteDatabricksCompatibility.java Outdated

Fix styles in Delta Lake documentation

6645a97

Add test environment for Databricks 12.2 LTS

86ea8ec

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from 813d33d to dcde28f Compare April 13, 2023 23:11

findinpath reviewed Apr 14, 2023

View reviewed changes

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from dcde28f to 464f999 Compare April 14, 2023 06:08

krvikash reviewed Apr 14, 2023

View reviewed changes

ebyhr added 2 commits April 14, 2023 19:13

Disallow reader version > 3 and unsupported features in Delta Lake

3d7fe32

Change DeltaLakeMetadata.MAX_WRITER_VERSION to private

ce76b63

ebyhr force-pushed the ebi/delta-databricks-12.2 branch from 464f999 to ce76b63 Compare April 14, 2023 10:29

findinpath mentioned this pull request Apr 14, 2023

Delta Lake connector returns incorrect results when table has deletion vectors #16884

Closed

empty

8fd326b

ebyhr merged commit 602cd0f into master Apr 16, 2023

ebyhr deleted the ebi/delta-databricks-12.2 branch April 16, 2023 23:32

ebyhr mentioned this pull request Apr 16, 2023

Release notes for 414 #17002

Closed

github-actions Bot added this to the 414 milestone Apr 16, 2023

findepi reviewed Apr 17, 2023

View reviewed changes

colebow mentioned this pull request Apr 19, 2023

Add Trino 414 release notes #17128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Databricks 12.2 LTS and disallow unsupported reader versions and features in Delta Lake#16905

Support Databricks 12.2 LTS and disallow unsupported reader versions and features in Delta Lake#16905
ebyhr merged 5 commits intomasterfrom
ebi/delta-databricks-12.2

ebyhr commented Apr 6, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findinpath left a comment

Uh oh!

Uh oh!

Uh oh!

findinpath Apr 14, 2023

Uh oh!

ebyhr Apr 14, 2023

Uh oh!

findinpath Apr 14, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findepi Apr 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

	ListObjectsV2Request request = new ListObjectsV2Request()
	.withBucketName(getBucketName(uri))
	.withPrefix(key)
	.withDelimiter(recursive ? null : PATH_SEPARATOR)

	// Ignore directories (and symlinks, etc.). If a partition directory is old and empty (or made empty by us), removing it could break concurrent writes
	// on file systems with directories (e.g. HDFS).
	// TODO Note: Databricks can delete directories during vacuum on s3. This might need to be revisited.

Conversation

ebyhr commented Apr 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Release notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findinpath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

findinpath Apr 14, 2023

Choose a reason for hiding this comment

Uh oh!

ebyhr Apr 14, 2023

Choose a reason for hiding this comment

Uh oh!

findinpath Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findepi Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

ebyhr commented Apr 6, 2023 •

edited

Loading

findinpath Apr 14, 2023 •

edited

Loading